That's the races out of our test suite! #5384

andydotxyz · 2025-01-09T14:56:46Z

Working perfectly locally, let's see what CI says!

Progresses #4654

Checklist:

Tests included.
Lint and formatter run with no errors.
Tests all pass.

…tion

Also actually run the mobile driver during tests

Fixes fyne-io#5375

andydotxyz · 2025-01-09T17:45:31Z

Oh man, so many races not detected on my local setup :(

dweymouth · 2025-01-09T18:10:06Z

InMemoryPreferences will need to avoid using a goroutine here

andydotxyz · 2025-01-09T18:12:57Z

InMemoryPreferences will need to avoid using a goroutine here

Good catch thanks. Fixed

andydotxyz · 2025-01-09T18:26:26Z

OK, some platforms are now not detecting any races, so I have enabled it on the rest and working through what I hope is the last few!!!

dweymouth · 2025-01-09T18:39:42Z

Use new RunFromGoroutine API here - https://github.com/fyne-io/fyne/blob/develop/app/settings_desktop.go#L48

coveralls · 2025-01-09T18:41:55Z

coverage: 59.759% (+0.4%) from 59.329%
when pulling 7d5fb07 on andydotxyz:fix/racedetection
into 1bfc24c on fyne-io:develop.

coveralls · 2025-01-09T18:42:03Z

coverage: 59.754% (+0.4%) from 59.329%
when pulling 367a6a6 on andydotxyz:fix/racedetection
into 1bfc24c on fyne-io:develop.

coveralls · 2025-01-09T18:42:04Z

coverage: 59.764% (+0.4%) from 59.329%
when pulling 895953c on andydotxyz:fix/racedetection
into 1bfc24c on fyne-io:develop.

coveralls · 2025-01-09T18:42:10Z

coverage: 59.228% (-0.1%) from 59.374%
when pulling 6369f2a on andydotxyz:fix/racedetection
into f2ba049 on fyne-io:develop.

dweymouth · 2025-01-09T19:27:35Z

@Jacalz It seems we may need to remove or update the new pool_test. I think sync.Pool doesn't make a guarantee that an object released into the pool is immediately available for re-use, which is why it's failing occasionally here

andydotxyz · 2025-01-09T20:28:03Z

@Jacalz It seems we may need to remove or update the new pool_test. I think sync.Pool doesn't make a guarantee that an object released into the pool is immediately available for re-use, which is why it's failing occasionally here

Good point, the contract includes this on Get:

// Get may choose to ignore the pool and treat it as empty.
// Callers should not assume any relation between values passed to Put and
// the values returned by Get.

So I have removed the Set/Get test - but the Get on empty and the replacement New tests should both be OK.

andydotxyz · 2025-01-10T16:11:37Z

This sleep looks suspicous:

fyne/app/preferences.go

Line 52 in 1bfc24c

time.Sleep(time.Millisecond * 100) // writes are not always atomic. 10ms worked, 100 is safer.

Thanks - fixed. I left the delay as there was a bug reported about file write delay but then it joins back to main now

…l app - that requires more CI work

dialog/file.go

andydotxyz · 2025-01-10T22:02:19Z

PASS!!!

Jacalz

Thanks. This is massively impressive. Well done! I left some notes and questions in-line :)

app/cloud_test.go

app/preferences.go

dialog/file.go

internal/driver/mobile/canvas_test.go

test/app.go

widget/entry_cursor_anim.go

Jacalz · 2025-01-11T10:06:14Z

I am getting a strange compile failure on Linux when building fyne_demo with -race now:

# fyne.io/fyne/v2/cmd/fyne_demo
/usr/lib/golang/pkg/tool/linux_amd64/link: running gcc failed: exit status 1
/usr/bin/gcc -m64 -o $WORK/b001/exe/a.out -Wl,--export-dynamic-symbol=_cgo_panic -Wl,--export-dynamic-symbol=_cgo_topofstack -Wl,--export-dynamic-symbol=crosscall2 -Wl,--export-dynamic-symbol=glowDebugCallback_gl21 -Wl,--export-dynamic-symbol=goCharCB -Wl,--export-dynamic-symbol=goCharModsCB -Wl,--export-dynamic-symbol=goCursorEnterCB -Wl,--export-dynamic-symbol=goCursorPosCB -Wl,--export-dynamic-symbol=goDropCB -Wl,--export-dynamic-symbol=goErrorCB -Wl,--export-dynamic-symbol=goFramebufferSizeCB -Wl,--export-dynamic-symbol=goJoystickCB -Wl,--export-dynamic-symbol=goKeyCB -Wl,--export-dynamic-symbol=goMonitorCB -Wl,--export-dynamic-symbol=goMouseButtonCB -Wl,--export-dynamic-symbol=goScrollCB -Wl,--export-dynamic-symbol=goWindowCloseCB -Wl,--export-dynamic-symbol=goWindowContentScaleCB -Wl,--export-dynamic-symbol=goWindowFocusCB -Wl,--export-dynamic-symbol=goWindowIconifyCB -Wl,--export-dynamic-symbol=goWindowMaximizeCB -Wl,--export-dynamic-symbol=goWindowPosCB -Wl,--export-dynamic-symbol=goWindowRefreshCB -Wl,--export-dynamic-symbol=goWindowSizeCB -Wl,--compress-debug-sections=zlib /tmp/go-link-2655855783/go.o /tmp/go-link-2655855783/000000.o /tmp/go-link-2655855783/000001.o /tmp/go-link-2655855783/000002.o /tmp/go-link-2655855783/000003.o /tmp/go-link-2655855783/000004.o /tmp/go-link-2655855783/000005.o /tmp/go-link-2655855783/000006.o /tmp/go-link-2655855783/000007.o /tmp/go-link-2655855783/000008.o /tmp/go-link-2655855783/000009.o /tmp/go-link-2655855783/000010.o /tmp/go-link-2655855783/000011.o /tmp/go-link-2655855783/000012.o /tmp/go-link-2655855783/000013.o /tmp/go-link-2655855783/000014.o /tmp/go-link-2655855783/000015.o /tmp/go-link-2655855783/000016.o /tmp/go-link-2655855783/000017.o /tmp/go-link-2655855783/000018.o /tmp/go-link-2655855783/000019.o /tmp/go-link-2655855783/000020.o /tmp/go-link-2655855783/000021.o /tmp/go-link-2655855783/000022.o /tmp/go-link-2655855783/000023.o /tmp/go-link-2655855783/000024.o /tmp/go-link-2655855783/000025.o /tmp/go-link-2655855783/000026.o /tmp/go-link-2655855783/000027.o /tmp/go-link-2655855783/000028.o /tmp/go-link-2655855783/000029.o /tmp/go-link-2655855783/000030.o /tmp/go-link-2655855783/000031.o /tmp/go-link-2655855783/000032.o /tmp/go-link-2655855783/000033.o /tmp/go-link-2655855783/000034.o /tmp/go-link-2655855783/000035.o /tmp/go-link-2655855783/000036.o /tmp/go-link-2655855783/000037.o /tmp/go-link-2655855783/000038.o /tmp/go-link-2655855783/000039.o /tmp/go-link-2655855783/000040.o /tmp/go-link-2655855783/000041.o /tmp/go-link-2655855783/000042.o /tmp/go-link-2655855783/000043.o /tmp/go-link-2655855783/000044.o /tmp/go-link-2655855783/000045.o /tmp/go-link-2655855783/000046.o /tmp/go-link-2655855783/000047.o -O2 -g -O2 -g -lresolv -O2 -g -lGL -lX11 -lXrandr -lXxf86vm -lXi -lXcursor -lm -lXinerama -ldl -lrt -O2 -g -lpthread -O2 -g -O2 -g -lGL -no-pie
/usr/bin/ld: /tmp/go-link-2655855783/go.o: in function `runtime.(*mspan).setUserArenaChunkToFault':
go.go:(.text+0x7202): undefined reference to `__tsan_free'
/usr/bin/ld: /tmp/go-link-2655855783/go.o: in function `runtime.runfinq':
/usr/lib/golang/src/runtime/race.go:585:(.text+0x19614): undefined reference to `__tsan_finalizer_goroutine'
/usr/bin/ld: /tmp/go-link-2655855783/go.o: in function `runtime.(*sweepLocked).sweep':
/usr/lib/golang/src/runtime/race.go:461:(.text+0x27191): undefined reference to `__tsan_free'
/usr/bin/ld: /tmp/go-link-2655855783/go.o: in function `runtime.goexit1':
/usr/lib/golang/src/runtime/race.go:481:(.text+0x4399d): undefined reference to `__tsan_go_end'
/usr/bin/ld: /tmp/go-link-2655855783/go.o: in function `runtime.gfget':
/usr/lib/golang/src/runtime/race.go:456:(.text+0x45b39): undefined reference to `__tsan_malloc'
/usr/bin/ld: /tmp/go-link-2655855783/go.o: in function `runtime.(*p).init':
/usr/lib/golang/src/runtime/race.go:434:(.text+0x4665f): undefined reference to `__tsan_proc_create'
/usr/bin/ld: /tmp/go-link-2655855783/go.o: in function `runtime.(*p).destroy':
/usr/lib/golang/src/runtime/race.go:486:(.text+0x469d2): undefined reference to `__tsan_go_end'
/usr/bin/ld: /usr/lib/golang/src/runtime/race.go:440:(.text+0x46a29): undefined reference to `__tsan_proc_destroy'
/usr/bin/ld: /tmp/go-link-2655855783/go.o: in function `runtime.RaceDisable':
/usr/lib/golang/src/runtime/race.go:72:(.text+0x4b2c1): undefined reference to `__tsan_go_ignore_sync_begin'
/usr/bin/ld: /tmp/go-link-2655855783/go.o: in function `runtime.RaceEnable':
/usr/lib/golang/src/runtime/race.go:84:(.text+0x4b34d): undefined reference to `__tsan_go_ignore_sync_end'
/usr/bin/ld: /tmp/go-link-2655855783/go.o: in function `runtime.raceinit':
/usr/lib/golang/src/runtime/race.go:373:(.text+0x4b9af): undefined reference to `__tsan_init'
/usr/bin/ld: /usr/lib/golang/src/runtime/race.go:403:(.text+0x4ba1c): undefined reference to `__tsan_map_shadow'
/usr/bin/ld: /tmp/go-link-2655855783/go.o: in function `runtime.racefini':
/usr/lib/golang/src/runtime/race.go:428:(.text+0x4bb1e): undefined reference to `__tsan_fini'
/usr/bin/ld: /tmp/go-link-2655855783/go.o: in function `runtime.racemapshadow':
/usr/lib/golang/src/runtime/race.go:451:(.text+0x4bb9f): undefined reference to `__tsan_map_shadow'
/usr/bin/ld: /tmp/go-link-2655855783/go.o: in function `runtime.racegostart':
/usr/lib/golang/src/runtime/race.go:475:(.text+0x4bc24): undefined reference to `__tsan_go_start'
/usr/bin/ld: /tmp/go-link-2655855783/go.o: in function `runtime.raceacquireg':
/usr/lib/golang/src/runtime/race.go:533:(.text+0x4be4a): undefined reference to `__tsan_acquire'
/usr/bin/ld: /tmp/go-link-2655855783/go.o: in function `runtime.raceacquirectx':
/usr/lib/golang/src/runtime/race.go:541:(.text+0x4beda): undefined reference to `__tsan_acquire'
/usr/bin/ld: /tmp/go-link-2655855783/go.o: in function `runtime.racereleaseg':
/usr/lib/golang/src/runtime/race.go:554:(.text+0x4bf6a): undefined reference to `__tsan_release'
/usr/bin/ld: /tmp/go-link-2655855783/go.o: in function `runtime.racereleaseacquireg':
/usr/lib/golang/src/runtime/race.go:567:(.text+0x4c00a): undefined reference to `__tsan_release_acquire'
/usr/bin/ld: /tmp/go-link-2655855783/go.o: in function `runtime.racereleasemergeg':
/usr/lib/golang/src/runtime/race.go:580:(.text+0x4c0aa): undefined reference to `__tsan_release_merge'
/usr/bin/ld: /tmp/go-link-2655855783/go.o: in function `runtime.stackalloc':
/usr/lib/golang/src/runtime/race.go:456:(.text+0x53d56): undefined reference to `__tsan_malloc'
/usr/bin/ld: /tmp/go-link-2655855783/go.o: in function `runtime.mallocgc':
/usr/lib/golang/src/runtime/race.go:456:(.text+0x6c32c): undefined reference to `__tsan_malloc'
/usr/bin/ld: /tmp/go-link-2655855783/go.o: in function `racefuncenter':
/usr/lib/golang/src/runtime/race_amd64.s:182:(.text+0x7b0b0): undefined reference to `__tsan_func_enter'
/usr/bin/ld: /tmp/go-link-2655855783/go.o: in function `racecallatomic':
/usr/lib/golang/src/runtime/race_amd64.s:410:(.text+0x7b10e): undefined reference to `__tsan_go_ignore_sync_begin'
/usr/bin/ld: /usr/lib/golang/src/runtime/race_amd64.s:421:(.text+0x7b13e): undefined reference to `__tsan_go_ignore_sync_end'
/usr/bin/ld: /tmp/go-link-2655855783/go.o: in function `runtime.raceread':
/usr/lib/golang/src/runtime/race_amd64.s:51:(.text+0x7b1aa): undefined reference to `__tsan_read'
/usr/bin/ld: /tmp/go-link-2655855783/go.o: in function `runtime.racereadpc.abi0':
/usr/lib/golang/src/runtime/race_amd64.s:66:(.text+0x7b1f6): undefined reference to `__tsan_read_pc'
/usr/bin/ld: /tmp/go-link-2655855783/go.o: in function `runtime.racewrite':
/usr/lib/golang/src/runtime/race_amd64.s:77:(.text+0x7b20a): undefined reference to `__tsan_write'
/usr/bin/ld: /tmp/go-link-2655855783/go.o: in function `runtime.racewritepc.abi0':
/usr/lib/golang/src/runtime/race_amd64.s:92:(.text+0x7b256): undefined reference to `__tsan_write_pc'
/usr/bin/ld: /tmp/go-link-2655855783/go.o: in function `runtime.racereadrange':
/usr/lib/golang/src/runtime/race_amd64.s:104:(.text+0x7b26d): undefined reference to `__tsan_read_range'
/usr/bin/ld: /tmp/go-link-2655855783/go.o: in function `runtime.racereadrangepc1.abi0':
/usr/lib/golang/src/runtime/race_amd64.s:119:(.text+0x7b2b6): undefined reference to `__tsan_read_range'
/usr/bin/ld: /tmp/go-link-2655855783/go.o: in function `runtime.racewriterange':
/usr/lib/golang/src/runtime/race_amd64.s:131:(.text+0x7b2cd): undefined reference to `__tsan_write_range'
/usr/bin/ld: /tmp/go-link-2655855783/go.o: in function `runtime.racewriterangepc1.abi0':
/usr/lib/golang/src/runtime/race_amd64.s:146:(.text+0x7b316): undefined reference to `__tsan_write_range'
/usr/bin/ld: /tmp/go-link-2655855783/go.o: in function `runtime.racefuncexit.abi0':
/usr/lib/golang/src/runtime/race_amd64.s:193:(.text+0x7b34a): undefined reference to `__tsan_func_exit'
/usr/bin/ld: /tmp/go-link-2655855783/go.o: in function `sync/atomic.StoreInt64.abi0':
/usr/lib/golang/src/runtime/race_amd64.s:236:(.text+0x7b363): undefined reference to `__tsan_go_atomic64_store'
/usr/bin/ld: /tmp/go-link-2655855783/go.o: in function `sync/atomic.SwapInt64.abi0':
/usr/lib/golang/src/runtime/race_amd64.s:261:(.text+0x7b3a3): undefined reference to `__tsan_go_atomic64_exchange'
/usr/bin/ld: /tmp/go-link-2655855783/go.o: in function `sync/atomic.AndInt32.abi0':
/usr/lib/golang/src/runtime/race_amd64.s:309:(.text+0x7b3e3): undefined reference to `__tsan_go_atomic32_fetch_and'
/usr/bin/ld: /tmp/go-link-2655855783/go.o: in function `sync/atomic.AndInt64.abi0':
/usr/lib/golang/src/runtime/race_amd64.s:315:(.text+0x7b403): undefined reference to `__tsan_go_atomic64_fetch_and'
/usr/bin/ld: /tmp/go-link-2655855783/go.o: in function `sync/atomic.OrInt32.abi0':
/usr/lib/golang/src/runtime/race_amd64.s:334:(.text+0x7b463): undefined reference to `__tsan_go_atomic32_fetch_or'
/usr/bin/ld: /tmp/go-link-2655855783/go.o: in function `sync/atomic.OrInt64.abi0':
/usr/lib/golang/src/runtime/race_amd64.s:340:(.text+0x7b483): undefined reference to `__tsan_go_atomic64_fetch_or'
/usr/bin/ld: /tmp/go-link-2655855783/go.o: in function `sync/atomic.CompareAndSwapInt64.abi0':
/usr/lib/golang/src/runtime/race_amd64.s:366:(.text+0x7b4e3): undefined reference to `__tsan_go_atomic64_compare_exchange'
collect2: error: ld returned 1 exit status

Will clean my caches and restart the computer to see if it is some corrupt thing.

Jacalz

Getting the same error on develop now for some reason. Anyway, this looks great 👍

andydotxyz added 11 commits January 7, 2025 15:50

Synchronise the theme watcher and other "background" items in test code

8412c9d

Refactored test package to not have races in the app settings applica…

d35da3e

…tion

Resolve a race in the preference code

5ae7d16

Remove lock complications in test, they now behave better

d28e1c5

Correct race in mobile double tap handling

145dd07

Also actually run the mobile driver during tests

Remove locks and fix thread safety of cursor animation

89f5f20

Move the window safety code into a wrapper type

b0e5c64

Fixes fyne-io#5375

Fixes to canvas tests and what looked like 2 bad test files

08d798a

The final runOnMains for testing, and enable race checks in CI!

00d741f

Merge branch 'develop' into fix/racedetection

4c0458c

Fix merge conflict

7d5fb07

andydotxyz force-pushed the fix/racedetection branch from d1bbb2a to 7d5fb07 Compare January 9, 2025 17:40

andydotxyz added 2 commits January 9, 2025 17:50

Always use safe canvas in tests

367a6a6

Fix race in animation tests

895953c

Don't use goroutines in preference applciation

13d1f77

andydotxyz added 2 commits January 9, 2025 18:16

Add race to mobile and wayland

4713739

Fixing some more window test races

a0ecb94

andydotxyz added 2 commits January 9, 2025 20:23

Missed another goroutine

98dc17d

Remove test that doesn't agree with pool contract

b97878b

More safety in preference write

fb997d8

andydotxyz added 3 commits January 10, 2025 16:33

Lighter weight mobile test - more than before this PR but not the ful…

afb5db5

…l app - that requires more CI work

Potential issue with preferences and mobile

52a15d9

Adjusting thread on file watcher callbacks

a446af3

andydotxyz self-assigned this Jan 10, 2025

andydotxyz and others added 12 commits January 10, 2025 17:11

Fix staticcheck

154d1e6

More GLFW fixes

c2f8169

Correct threading of cloud apply test

2b9f46d

Don't "find" app files for dialog tests

b37b4a5

Fix a race in menu bar tests on Ubuntu

df498a6

Menu item safety for Ubuntu tests too

49cbf18

GLFW tests need real main thread

190d747

Remove last worrying goroutines

f04b0c9

Same fix avoiding import loop

6cec612

Safer tapping around in GLFW tests

1ae8045

Protecting more complex tap flows

bcb6742

Passing locally on Linux now too

15b454e

dweymouth reviewed Jan 10, 2025

View reviewed changes

dialog/file.go Show resolved Hide resolved

andydotxyz added 2 commits January 10, 2025 21:42

Fixing some intermittent

7dbcac0

Fixing another occasional race

62d9f9a

Jacalz requested changes Jan 10, 2025

View reviewed changes

andydotxyz added 2 commits January 11, 2025 09:32

Tidying from PR feedback

96a595f

Correct done pool usage

6369f2a

Jacalz approved these changes Jan 11, 2025

View reviewed changes

andydotxyz merged commit 2355f9d into fyne-io:develop Jan 11, 2025
12 checks passed

andydotxyz deleted the fix/racedetection branch January 11, 2025 10:25

andydotxyz mentioned this pull request Jan 11, 2025

New threading model #4654

Open

16 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

That's the races out of our test suite! #5384

That's the races out of our test suite! #5384

andydotxyz commented Jan 9, 2025

andydotxyz commented Jan 9, 2025

dweymouth commented Jan 9, 2025

andydotxyz commented Jan 9, 2025

andydotxyz commented Jan 9, 2025

dweymouth commented Jan 9, 2025

coveralls commented Jan 9, 2025

coveralls commented Jan 9, 2025

coveralls commented Jan 9, 2025

coveralls commented Jan 9, 2025 •

edited

Loading

dweymouth commented Jan 9, 2025

andydotxyz commented Jan 9, 2025

andydotxyz commented Jan 10, 2025

andydotxyz commented Jan 10, 2025

Jacalz left a comment

Jacalz commented Jan 11, 2025

Jacalz left a comment

That's the races out of our test suite! #5384

That's the races out of our test suite! #5384

Conversation

andydotxyz commented Jan 9, 2025

Checklist:

andydotxyz commented Jan 9, 2025

dweymouth commented Jan 9, 2025

andydotxyz commented Jan 9, 2025

andydotxyz commented Jan 9, 2025

dweymouth commented Jan 9, 2025

coveralls commented Jan 9, 2025

coveralls commented Jan 9, 2025

coveralls commented Jan 9, 2025

coveralls commented Jan 9, 2025 • edited Loading

dweymouth commented Jan 9, 2025

andydotxyz commented Jan 9, 2025

andydotxyz commented Jan 10, 2025

andydotxyz commented Jan 10, 2025

Jacalz left a comment

Choose a reason for hiding this comment

Jacalz commented Jan 11, 2025

Jacalz left a comment

Choose a reason for hiding this comment

coveralls commented Jan 9, 2025 •

edited

Loading